Allow for changing the default merging strategy. #53
Conversation
|
Thanks @mmulqueen, this is a nice improvement. But I would rather configure the default strategy in salt config instead of the URL parameter style config which is actually a bit hackish. (Salt-master config should be available in |
| @@ -120,22 +121,25 @@ def _merge_dict(stack, obj): | |||
| stack_k = stack[k] | |||
| stack[k] = _cleanup(v) | |||
| v = stack_k | |||
| # Stop it getting double inverted later on. | |||
| if default_strategy == "merge-first": | |||
| default_strategy = "merge-last" | |||
There was a problem hiding this comment.
what's the purpose of the above lines?
|
It is because pillarstack reverses the stack with merge-first by switching
the two dicts. This is fine, but causes odd behaviour for nested dicts when
given a different default. When specified with double underscores this
doesn't happen because the double underscore is only evaluated on one level
rather than every level.
Not sure whether I've explained that well enough?
|
|
@bbinet No rush at all, but is there any more info/explanation you need on this from me? |
|
@bbinet We've been successfully using this in prod all summer and it seems to be working well and as we'd expect. Is there anything else I can do to help complete this pull request? |
|
Sorry @mmulqueen for my silence. Also, I don't really like the overwrite of the default_strategy variable: I think this could be easily avoided if Does it make sense? What do you think? |
|
For the lists, it was actually a "default strategy", not only "top level strategy" so this is not consistent with the dicts... |
We have been using Pillarstack for a week or so now and it's great, it's a real improvement from Salt's built in pillar, but still quite simple. For us, it's made sense to use a merge-first strategy throughout - it gives us layers of increasingly generic defaults.
I found that I was having to put
__: merge-firstin all the files though and I kept forgetting. This is a patch to be able to change the default and take the human error out of the equation.Configured in the salt master config like this:
Hopefully this is a useful feature. I feel like the way of configuring it is a bit hackish, but considering how you're already using args, kwargs in ext_pillar, I thought this was the cleanest way of doing it. Or could perhaps switch to a dict style instead of URL parameter style.